Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix for scrolling content #112

Merged
merged 1 commit into from
Sep 11, 2022
Merged

Fix for scrolling content #112

merged 1 commit into from
Sep 11, 2022

Conversation

tchudyk
Copy link
Contributor

@tchudyk tchudyk commented Sep 11, 2022

Probably, after commit [https://github.com//pull/106] I have noticed a negative impact on RichTextFx - during typing text in editor it starts "minimally" scrolling.

I see, that in mentioned commit there is Math.round(offset) in VirtualizedScrollPane:

   private void setVPosition(double pos) {
        // ....
        content.estimatedScrollYProperty().setValue((double) Math.round(offset));
    }

But farther, when this action goes to VirtualFlow class - to method:

void setLengthOffset(double pixels) {
        double total = totalLengthEstimateProperty().getOrElse(0.0);
        double length = sizeTracker.getViewportLength();
        double max = Math.max(total - length, 0);
        double current = lengthOffsetEstimate.getValue();

        if(pixels > max) pixels = max;
        if(pixels < 0) pixels = 0;

        double diff = pixels - current;
        if(diff == 0) {
            // do nothing
        } else if(Math.abs(diff) <= length) { // distance less than one screen
            navigator.scrollCurrentPositionBy(diff);
        } else {
            jumpToAbsolutePosition(pixels);
        }
    }

The incoming pixels value is rounded, but value in variable current is not rounded, and when we compute diff:

double diff = pixels - current;

... there is very often computed value 0.5 (but it should be 0) and VirtualFlow moves visible content by this diff (navigator.scrollCurrentPositionBy(diff)) instead of // do nothing

My fix here is to apply rounding on lengthOffsetEstimate. This solves this issue.

@Jugen
Copy link
Contributor

Jugen commented Sep 11, 2022

@tchudyk Thank you very much for your analysis and PR request. It's very much appreciated. Will merge ....

@Jugen Jugen merged commit b5ef653 into FXMisc:master Sep 11, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants